Skip to content

fix: derive client IP from trusted proxy hops only - #1399

Open
devflora-princess wants to merge 4 commits into
CalloraOrg:mainfrom
devflora-princess:security/issue-1268-derive-client-ips-from-trusted-proxy-hops-only
Open

devflora-princess wants to merge 4 commits into
CalloraOrg:mainfrom
devflora-princess:security/issue-1268-derive-client-ips-from-trusted-proxy-hops-only

Conversation

@devflora-princess

@devflora-princess devflora-princess commented Sep 29, 2026 •

Copy link
Copy Markdown

Overview

This PR replaces the boolean TRUST_PROXY_HEADERS behavior in getClientIp with a trusted-hop-count model that selects the client IP from the right side of X-Forwarded-For, matching Express trust proxy semantics. The leftmost XFF entry is fully client-controlled, so the previous behavior allowed spoofing past the admin IP allowlist and per-IP rate limits. The change keeps the default (no trust) behavior of using the socket address, and updates the allowlist middleware and forwarded-header policy doc to reflect the new invariant.

Related Issue

Changes

🔒 Trusted-hop client IP resolution

  • [MODIFY] src/lib/clientIp.ts

    • getClientIp now takes a trusted hop count instead of a boolean.
    • Selects the entry hops positions from the right of X-Forwarded-For, so with one trusted hop 1.1.1.1, 2.2.2.2 yields 2.2.2.2.
    • Falls back to the socket address when trust is disabled, the header is missing/malformed, or the hop count exceeds the available entries.
    • Leftmost (client-supplied) entries are never selected when trust is configured.
  • [MODIFY] src/config/env.ts

    • Replaces the boolean trust flag with a numeric trusted-hop count parsed from env, defaulting to 0 (no trust → socket address).
  • [MODIFY] src/middleware/ipAllowlist.ts

    • Passes the configured hop count through to getClientIp so the allowlist decision is made on the trusted-hop-derived IP rather than the spoofable leftmost value.
  • [MODIFY] FORWARDED_HEADER_POLICY.md

    • Documents the trusted-hop model, the right-to-left selection rule, the default no-trust behavior, and the Express trust proxy alignment.
  • [MODIFY] src/lib/__tests__/clientIp.test.ts

    • Updated to cover the new hop-based signature and the acceptance criteria below.

Verification Results

npm test -- src/lib/__tests__/clientIp.test.ts tests/integration/ipAllowlist.integration.test.ts
Acceptance Criteria Status
With one trusted hop, X-Forwarded-For: 1.1.1.1, 2.2.2.2 yields 2.2.2.2 ✅ Right-to-left hop selection in getClientIp
Spoofed leftmost entries cannot satisfy the admin allowlist ✅ ipAllowlist uses the trusted-hop-derived IP
Default (no trust) still uses the socket address ✅ Hop count defaults to 0; socket address used
src/lib/__tests__/clientIp.test.ts is updated ✅ Tests updated for hop-based behavior and fallbacks

Security and Failure Modes

  • Spoofing: The leftmost XFF entry is no longer trusted; only entries at the configured hop depth from the right are considered, so client-injected prefixes cannot influence the resolved IP.
  • Misconfiguration: If the hop count exceeds the number of XFF entries, the resolver falls back to the socket address rather than guessing, avoiding silent trust of attacker-controlled values.
  • Default safety: With no trust configured (0 hops), behavior matches the pre-existing no-trust path and uses the socket address.
  • Compatibility: Callers that previously passed a boolean must now pass a hop count; the env var is parsed as a number with a safe default of 0.

Non-goals

  • No typo-only, formatting-only, or cosmetic changes.
  • No unrelated refactors, dependency upgrades, or broad rewrites.
  • No removal of safeguards or weakening of validation.

Closes #1268

@drips-wave

drips-wave Bot commented Sep 29, 2026

Copy link
Copy Markdown

@devflora-princess Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@devflora-princess devflora-princess changed the title fix: derive client IP from trusted proxy hops only fix: derive client IPs from trusted proxy hops only Sep 29, 2026
@devflora-princess devflora-princess changed the title fix: derive client IPs from trusted proxy hops only fix: derive client IP from trusted proxy hops only Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Derive client IPs from trusted proxy hops only

1 participant